Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add startupProbe and replace readiness probe with liveness probe #5407

Merged
merged 52 commits into from
Dec 7, 2022
Merged

feat: add startupProbe and replace readiness probe with liveness probe #5407

merged 52 commits into from
Dec 7, 2022

Conversation

JoanFM
Copy link
Member

@JoanFM JoanFM commented Nov 17, 2022 •
edited by girishc13
Loading

Goals:
Now, while Requests are being processed, the event loop of Worker is blocked and readinessProbe can start failing if Requests block the event loop too long (Executor methods taking long)

Resolves #5470

@github-actions github-actions bot added size/XS area/core This issue/PR affects the core codebase area/helper This issue/PR affects the helper functionality labels Nov 17, 2022
@codecov
Copy link

codecov bot commented Nov 17, 2022 •
edited
Loading

Codecov Report

Merging #5407 (b2437d8) into master (fbac885) will increase coverage by 17.11%.
The diff coverage is 80.00%.

@@             Coverage Diff             @@
##           master    #5407       +/-   ##
===========================================
+ Coverage   68.77%   85.88%   +17.11%     
===========================================
  Files          99      101        +2     
  Lines        6680     6724       +44     
===========================================
+ Hits         4594     5775     +1181     
+ Misses       2086      949     -1137
Flag Coverage Δ
jina 85.88% <80.00%> (+17.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jina/serve/networking.py 88.88% <ø> (+17.93%) ⬆️
jina/checker.py 85.36% <50.00%> (+85.36%) ⬆️
jina/clients/base/helper.py 83.06% <100.00%> (+9.67%) ⬆️
jina/serve/runtimes/asyncio.py 86.48% <100.00%> (+10.38%) ⬆️
jina/serve/runtimes/gateway/composite/gateway.py 33.33% <0.00%> (-66.67%) ⬇️
jina/jaml/parsers/flow/v1.py 97.01% <0.00%> (-2.99%) ⬇️
...a/orchestrate/deployments/config/docker_compose.py 94.60% <0.00%> (-2.46%) ⬇️
jina/orchestrate/deployments/config/k8s.py 99.38% <0.00%> (ø)
jina/resources/health_check/gateway.py 41.17% <0.00%> (ø)
jina/resources/health_check/pod.py 50.00% <0.00%> (ø)
... and 62 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@JoanFM
fix: run health servicer in thread to allow readinessProbe while req
32dcbc2 
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
@JoanFM JoanFM force-pushed the health_servicer_thread branch from 394069a to 32dcbc2 Compare November 17, 2022 15:44
@github-actions github-actions bot added size/S area/cicd This issue/PR affects the cicd pipeline area/housekeeping This issue/PR is housekeeping area/testing This issue/PR affects testing labels Nov 17, 2022
@JoanFM JoanFM marked this pull request as ready for review November 17, 2022 16:24
@JoanFM JoanFM force-pushed the health_servicer_thread branch 3 times, most recently from b82180b to 75c88b8 Compare November 17, 2022 18:24
@JoanFM
test: add test with slow process
89d9645 
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
@JoanFM JoanFM force-pushed the health_servicer_thread branch from 75c88b8 to 89d9645 Compare November 17, 2022 20:35
@JoanFM
Copy link
Member Author

JoanFM commented Nov 18, 2022

I will add another test for this

@JoanFM
test: add test proving readinessProbe can pass while processing
308a3fa 
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
@JoanFM JoanFM force-pushed the health_servicer_thread branch from edcb1f1 to 308a3fa Compare November 21, 2022 10:33
@github-actions github-actions bot added area/cli This issue/PR affects the command line interface area/docker This issue/PR affects the docker functionality area/setup This issue/PR affects setting up Jina labels Nov 25, 2022
@github-actions github-actions bot removed area/cli This issue/PR affects the command line interface area/setup This issue/PR affects setting up Jina labels Dec 1, 2022
@girishc13 girishc13 self-assigned this Dec 1, 2022
@girishc13 girishc13 changed the title feat: add livenessProbe and startupProbe feat: add startupProbe Dec 1, 2022
@girishc13 girishc13 changed the title feat: add startupProbe feat: add startupProbe and replace readiness probe with liveness probe Dec 1, 2022
@girishc13 girishc13 marked this pull request as ready for review December 7, 2022 14:42
@github-actions github-actions bot removed the area/docker This issue/PR affects the docker functionality label Dec 7, 2022
@JoanFM JoanFM merged commit 974f688 into master Dec 7, 2022
@JoanFM JoanFM deleted the health_servicer_thread branch December 7, 2022 15:17
@alexcg1 alexcg1 mentioned this pull request Dec 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@girishc13 girishc13 girishc13 approved these changes

@deepankarm deepankarm Awaiting requested review from deepankarm

@hanxiao hanxiao Awaiting requested review from hanxiao

@nan-wang nan-wang Awaiting requested review from nan-wang

Assignees

@girishc13 girishc13

Labels
area/core This issue/PR affects the core codebase area/helper This issue/PR affects the helper functionality area/testing This issue/PR affects testing component/client component/resource size/L size/M size/S size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: add start up probe and replace readiness probe with liveness probe
3 participants
@JoanFM @girishc13 @jina-bot